Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Spearbit Audit Changes #25

Merged
merged 5 commits into from
Aug 22, 2024
Merged

Spearbit Audit Changes #25

merged 5 commits into from
Aug 22, 2024

Conversation

hexonaut
Copy link
Contributor

@hexonaut hexonaut commented Aug 21, 2024

Changes:

  • Remove the no-op role admin grants
  • Merge duplicate min grace period check
  • Spacing nit
  • Removed unused param in _executeTransaction
  • Better wording on IExecutor comment

Copy link

Coverage after merging spearbit-fixes into master will be

100.00%

Coverage Report
FileStmtsBranchesFuncsLinesUncovered Lines
src
   Executor.sol100%100%100%100%

Copy link
Contributor

@lucas-manuel lucas-manuel left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM pending spearbit confirmation that audit is finished and that the don't want to do issue based PRs

if (
gracePeriod_ < MINIMUM_GRACE_PERIOD
) revert InvalidInitParams();

_updateDelay(delay_);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original comment here

@@ -191,7 +182,7 @@ contract Executor is IExecutor, AccessControl {
function getCurrentState(uint256 actionsSetId) public view override returns (ActionsSetState) {
if (actionsSetCount <= actionsSetId) revert InvalidActionsSetId();

ActionsSet storage actionsSet =_actionsSets[actionsSetId];
ActionsSet storage actionsSet = _actionsSets[actionsSetId];
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original comment here

@@ -208,7 +199,6 @@ contract Executor is IExecutor, AccessControl {
uint256 value,
string memory signature,
bytes memory data,
uint256 executionTime,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original comment here

Copy link
Contributor

@barrutko barrutko left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@hexonaut hexonaut merged commit 5c16676 into master Aug 22, 2024
3 checks passed
@hexonaut hexonaut deleted the spearbit-fixes branch August 22, 2024 18:10
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants